Use eye coordinates for voxel raymarching #12933
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 29 commits
July 29, 2025 09:48
|
Thank you for the pull request, @jjhembd! ✅ We can confirm we have a CLA on file for you. |
ggetz
reviewed
Sep 30, 2025
| float clipAmount; | ||
| float pixelWidth = czm_metersPerPixel(position); | ||
| bool breakAndDiscard = false; | ||
| for (int i = 0; i < ${clippingPlanesLength}; ++i) |
There was a problem hiding this comment.
Thanks for cleaning this up! Should this (either here or a later PR) be adjusted to use compiler define statements?
Comment on lines
50
to
65
| /** | ||
| * An event triggered when a new clipping plane is added to the collection. Event handlers | ||
| * are passed the new plane and the index at which it was added. | ||
| * @type {Event} | ||
| * @default Event() | ||
| */ | ||
| this.planeAdded = new Event(); | ||
|
|
||
| /** | ||
| * An event triggered when a new clipping plane is removed from the collection. Event handlers | ||
| * are passed the new plane and the index from which it was removed. | ||
| * @type {Event} | ||
| * @default Event() | ||
| */ | ||
| this.planeRemoved = new Event(); | ||
|
|
There was a problem hiding this comment.
I think these should both be readonly, and therefore the default should be extraneous.
Suggested change
| /** | |
| * An event triggered when a new clipping plane is added to the collection. Event handlers | |
| * are passed the new plane and the index at which it was added. | |
| * @type {Event} | |
| * @default Event() | |
| */ | |
| this.planeAdded = new Event(); | |
| /** | |
| * An event triggered when a new clipping plane is removed from the collection. Event handlers | |
| * are passed the new plane and the index from which it was removed. | |
| * @type {Event} | |
| * @default Event() | |
| */ | |
| this.planeRemoved = new Event(); | |
| /** | |
| * An event triggered when a new clipping plane is added to the collection. Event handlers | |
| * are passed the new plane and the index at which it was added. | |
| * @type {Event} | |
| * @readonly | |
| */ | |
| this.planeAdded = new Event(); | |
| /** | |
| * An event triggered when a new clipping plane is removed from the collection. Event handlers | |
| * are passed the new plane and the index from which it was removed. | |
| * @type {Event} | |
| * @readonly | |
| */ | |
| this.planeRemoved = new Event(); | |
ggetz
requested changes
Oct 1, 2025
| } | ||
|
|
||
| vec3 scaleShapeUvToShapeSpace(in vec3 shapeUv) { | ||
| // TODO: scaling is wrong! |
There was a problem hiding this comment.
I think you mentioned offline that this is TODO is on your radar–A reminder to address here or to open an issue.
| vec3 rtu = u_cylinderEcToRadialTangentUp * positionEC; | ||
| // 2. Compute change in angular and radial coordinates. TODO: scaling camera is wrong! | ||
| vec2 dPolar = computePolarChange(rtu.xy, u_cameraShapePosition.x * u_cylinderWorldToLocalScale.x); | ||
| // TODO: this is wrong! Scaling needs to be applied in local XYZ coordinates |
|
I did some basic testing on existing voxel sandcastles and everything I observed matches |
lukemckinstry
approved these changes
Oct 1, 2025
ggetz
approved these changes
Oct 1, 2025
|
@jjhembd I'm now noticing a failing test in 1) picks a voxel cell from a VoxelPrimitive
Scene/Pick pickVoxel
RuntimeError: Fragment shader failed to compile. Compile log: ERROR: 0:89: '<<' : bit-wise operator supported in GLSL ES 3.00 and above only
ERROR: 0:174: '>>' : bit-wise operator supported in GLSL ES 3.00 and above only
ERROR: 0:177: '&' : bit-wise operator supported in GLSL ES 3.00 and above only
ERROR: 0:219: '>>' : bit-wise operator supported in GLSL ES 3.00 and above only
ERROR: 0:690: '<<' : bit-wise operator supported in GLSL ES 3.00 and above only
ERROR: 0:697: 'max' : no matching overloaded function found
ERROR: 0:697: '<<' : bit-wise operator supported in GLSL ES 3.00 and above only
ERROR: 0:697: 'min' : no matching overloaded function found
ERROR: 0:697: 'assign' : cannot convert from 'const mediump float' to 'highp 3-component vector of int'
error properties: undefined: undefined
Error
at new RuntimeError (packages/engine/Source/Core/RuntimeError.js:38:11 <- Build/CesiumUnminified/Cesium.js:11184:13)
at createAndLinkProgram (packages/engine/Source/Renderer/ShaderProgram.js:266:9 <- Build/CesiumUnminified/Cesium.js:26828:11)
at reinitialize (packages/engine/Source/Renderer/ShaderProgram.js:470:19 <- Build/CesiumUnminified/Cesium.js:26981:21)
at initialize2 (packages/engine/Source/Renderer/ShaderProgram.js:463:3 <- Build/CesiumUnminified/Cesium.js:26976:5)
at ShaderProgram._bind (packages/engine/Source/Renderer/ShaderProgram.js:542:3 <- Build/CesiumUnminified/Cesium.js:27035:5)
at beginDraw (packages/engine/Source/Renderer/Context.js:1304:17 <- Build/CesiumUnminified/Cesium.js:41212:19)
at Context.draw (packages/engine/Source/Renderer/Context.js:1414:3 <- Build/CesiumUnminified/Cesium.js:41300:5)
at DrawCommand.execute (packages/engine/Source/Renderer/DrawCommand.js:642:11 <- Build/CesiumUnminified/Cesium.js:23965:13)
at executeCommand (packages/engine/Source/Scene/Scene.js:2281:13 <- Build/CesiumUnminified/Cesium.js:229053:15)
at performVoxelsPass (packages/engine/Source/Scene/Scene.js:2407:5 <- Build/CesiumUnminified/Cesium.js:229120:7) |
8 tasks
6 tasks
This was referenced Oct 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR reworks the voxel shaders to perform the raymarching in eye-space coordinates, rather than model-space. This avoids precision problems when zoomed in close to small samples in a large model space.
Most of the precision problem came from the sampling of the voxel. The traversal in
Octree.glslhas been re-worked to input a two-part coordinate:[x, y, z, w]indices of the tile within the tileset[0, 1]indicating thex, y, zpositions within the tileThis new input to the traversal effectively gives us nearly double precision when sampling a large dataset.
Some parts of the code are still in a local model-centered coordinate system:
The latter 2 items could also be converted to eye coordinates with additional work. I did not do that work in this PR, mostly because of time constraints. I also haven't observed significant precision issues with those intersections, so that work does not seem urgent at this time.
Issue number and link
Fixes #12061
Testing plan
main.Author checklist
CONTRIBUTORS.mdCHANGES.mdwith a short summary of my change